Implement the experimental GroupingLatestValueCache
#428
Conversation
github-actions
bot
added
part:docs
There was a problem hiding this comment.
Pull Request Overview
This PR introduces an experimental cache – the GroupingLatestValueCache – which extends the existing caching functionality by grouping incoming messages by a key derived via a provided function. Key changes include:
- Addition of a new implementation under src/frequenz/channels/experimental/_grouping_latest_value_cache.py.
- Introduction of integration tests for the new cache in tests/test_grouping_latest_value_cache_integration.py.
- Updates to release notes and removal of keyed tests from the LatestValueCache integration test.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_latest_value_cache_integration.py | Removed keyed tests to separate concerns with the experimental cache implementation. |
| tests/test_grouping_latest_value_cache_integration.py | Added integration tests for validating the experimental grouping cache functionality. |
| src/frequenz/channels/experimental/_grouping_latest_value_cache.py | Added a new cache class that groups received values by key and caches the latest value per key. |
| src/frequenz/channels/experimental/init.py | Updated the module exports to include GroupingLatestValueCache. |
| src/frequenz/channels/_latest_value_cache.py | Removed support for the key parameter to align with the new, experimental behavior. |
| RELEASE_NOTES.md | Mentioned the new experimental GroupingLatestValueCache feature. |
Comments suppressed due to low confidence (1)
src/frequenz/channels/experimental/_grouping_latest_value_cache.py:78
- Consider adding a 'stop' method (similar to the one in LatestValueCache) to cancel the background task and ensure proper cleanup of resources.
self._task: asyncio.Task[None] = asyncio.create_task(
…ceived (frequenz-floss#425)" This reverts commit 6f24b25, reversing changes made to 3edcb49. Signed-off-by: Sahas Subramanian <[email protected]>
…s#424)" This reverts commit 3edcb49, reversing changes made to f7fb341. Signed-off-by: Sahas Subramanian <[email protected]>
shsms
force-pushed
the
GroupingLatestValueCache
branch
from
575bd46 to
0fedafe
Compare
shsms
requested review from
ela-kotulska-frequenz and
llucax
and removed request for
florian-wagner-frequenz
| def clear(self, key: HashableT) -> None: | ||
| """Clear the latest value or the latest value for a specific key. | ||
|
|
||
| Args: | ||
| key: The key for which to clear the latest value. | ||
| """ | ||
| _ = self._latest_value_by_key.pop(key, None) |
There was a problem hiding this comment.
Probably not udpated documentation: "Clear the latest value for a specific key"
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
Signed-off-by: Sahas Subramanian <[email protected]>
shsms
force-pushed
the
GroupingLatestValueCache
branch
from
0fedafe to
7597e34
Compare
There was a problem hiding this comment.
I was stating to write a lot of comments about using __contains__ instead of has_key, etc. but then probably we should just inherit from Mapping so we are required to follow the mapping interface, I think it is a perfect fit, this is basically a dict that is mutable/filled only externally by the passed receiver.
| ValueError: If no value has been received yet. | ||
| """ | ||
| if key not in self._latest_value_by_key: | ||
| raise ValueError(f"No value received for key: {key!r}") |
There was a problem hiding this comment.
Any reason why this is not a KeyError? 🤔
That's a good idea, I'll try. |
3 participants
It is similar to the
LatestValueCache, but accepts an additional key-function as an argument, which takes each incoming message and returns a key for that message. The latest value received for each unique key gets cached and is available to look up on demand.This also reverts the (unreleased) group-by-key extensions made to
LatestValueCache, so that there is more time to finalise the API before making it stable.